Skip to content

Fine-grained: Support Python 3 unpacking expressions (and fix crash) - #4966

Merged
JukkaL merged 4 commits into
masterfrom
fg-unpacking
Apr 26, 2018
Merged

JukkaL merged 4 commits into
masterfrom
fg-unpacking

Conversation

@JukkaL

@JukkaL JukkaL commented Apr 25, 2018

Copy link
Copy Markdown
Collaborator

Also fix crash on incompatible dictionary unpack. Fixes #4959.

Work towards #4951.

Also fix crash on incompatible dictionary unpack. Fixes #4959.

Work towards #4951.

@ilevkivskyi ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two questions:

  • Why do you skip cache in all tests? It would be good to add a comment about this in tests.
  • There is a TODO item in deps.py after process_lvalue (presumably left by you) that says # TODO: star lvalue. Isn't it something that should be addressed in this PR?

@JukkaL

JukkaL commented Apr 25, 2018

Copy link
Copy Markdown
Collaborator Author

Why do you skip cache in all tests? It would be good to add a comment about this in tests.

I'm skipping because many of the cache tests aren't very valuable but they'll slow down tests. I'll add a comment about this somewhere.

There is a TODO item in deps.py after process_lvalue (presumably left by you) that says # TODO: star lvalue. Isn't it something that should be addressed in this PR?

I'll have a look.

@JukkaL JukkaL changed the title Fine-grained: Add tests for unpacking expressions (and fix crash) Fine-grained: Support Python 3 unpacking expressions (and fix crash) Apr 26, 2018

@msullivan msullivan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JukkaL
JukkaL merged commit 7b257c8 into master Apr 26, 2018
@msullivan
msullivan deleted the fg-unpacking branch April 26, 2018 17:56
@asottile

Copy link
Copy Markdown
Contributor

I suspect this also fixes #3825

@emmatyping

Copy link
Copy Markdown
Member

@asottile, no I just tested and that also fails on master.

@asottile

Copy link
Copy Markdown
Contributor

indeed it does, I should try things before pressing [comment] 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash on incompatible dictionary unpack

5 participants